Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression with formatting toolbar #7167

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Jun 6, 2018

The right-border on the alignments was missing. This PR changes how we paint separators, so it gets fixed.

Verify this paints the right grouping borders for toolbar sections, both on mobile and desktop breakpoints, and with both the top-fixed toolbar and the block-fixed toolbar.

screen shot 2018-06-06 at 11 01 18


screen shot 2018-06-06 at 11 01 37


screen shot 2018-06-06 at 11 01 44

The right-border on the alignments was missing. This PR changes how we paint separators, so it gets fixed.

Verify this paints the right grouping borders for toolbar sections, both on mobile and desktop breakpoints, and with both the top-fixed toolbar and the block-fixed toolbar.
@jasmussen jasmussen added the [Type] Bug An existing feature does not function as intended label Jun 6, 2018
@jasmussen jasmussen self-assigned this Jun 6, 2018
@jasmussen jasmussen requested a review from a team June 6, 2018 09:02
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the before scenario for this? Without an issue filed that this fixes or before screenshots it's hard to tell what this actually fixes… but the screenshots here look alright so I'll say okay.

But in the future if there's no issue, before screenshots would be grand 😁

@jasmussen
Copy link
Contributor Author

Thanks for the review, and good thoughts. Here's what's in master:

screen shot 2018-06-06 at 12 56 43

Note the missing separator between alignments and inline.

CC: @mcsf in case you would like to merge this.

@jasmussen
Copy link
Contributor Author

Fixes #7028

Cc @aduth

@mcsf mcsf merged commit 5984cdf into master Jun 6, 2018
@mcsf mcsf deleted the fix/toolbar-border-regression branch June 6, 2018 14:46
@ZebulanStanphill
Copy link
Member

I think this should be added to the 3.1 milestone?

@jasmussen jasmussen added this to the 3.1 milestone Jun 7, 2018
@jasmussen
Copy link
Contributor Author

Good catch, Zeb. It might actually go in a 3.0.2 milestone, but milestoning it 3.1 for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants